Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce execution_hint for Cardinality aggregation (#17312) - backport to 2.19 #17420

Merged

Conversation

asimmahmood1
Copy link
Contributor

Description

Backport #17312 to 2.19

There's another PR for 2.x: #17419

Also need to backport the documentation opensearch-project/documentation-website#9224

Related Issues

Resolves #[16837]

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

…oject#17312) - backport to 2.19

---------

Signed-off-by: Siddharth Rayabharam <[email protected]>
Signed-off-by: Asim Mahmood <[email protected]>
Signed-off-by: Asim M <[email protected]>
Co-authored-by: Siddharth Rayabharam <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
(cherry picked from commit e3a6cca)
Signed-off-by: Asim Mahmood <[email protected]>
@peterzhuamazon peterzhuamazon marked this pull request as draft February 21, 2025 21:00
@peterzhuamazon
Copy link
Member

Convert this back to draft until further notice.
To understand if it will break anything in 2.19.1.

Thanks.

Copy link
Contributor

✅ Gradle check result for c7c3da1: SUCCESS

Copy link

codecov bot commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.

Project coverage is 71.97%. Comparing base (b006c0f) to head (c7c3da1).
Report is 9 commits behind head on 2.19.

Files with missing lines Patch % Lines
...gations/metrics/CardinalityAggregationBuilder.java 66.66% 1 Missing and 4 partials ⚠️
...egations/metrics/CardinalityAggregatorFactory.java 80.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               2.19   #17420      +/-   ##
============================================
- Coverage     72.00%   71.97%   -0.04%     
  Complexity    65963    65963              
============================================
  Files          5341     5341              
  Lines        307158   307179      +21     
  Branches      44824    44832       +8     
============================================
- Hits         221184   221081     -103     
- Misses        67498    67639     +141     
+ Partials      18476    18459      -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@asimmahmood1 asimmahmood1 marked this pull request as ready for review February 21, 2025 21:59
@asimmahmood1
Copy link
Contributor Author

| To understand if it will break anything in 2.19.1.

These changes are behind a new optional requset param for cardinality agg requests only. Without the new param, there is no change. There is a new response output, but its gated behind version. This in PR only nodes >= 2.19.1 will try to pass it.

@andrross andrross merged commit 4e7d103 into opensearch-project:2.19 Feb 21, 2025
47 checks passed
asimmahmood1 added a commit to asimmahmood1/OpenSearch that referenced this pull request Feb 21, 2025
* since 2.19 version has been merged:
  opensearch-project#17420

Signed-off-by: Asim Mahmood <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants